[mypyc] fix IntEnum attributes not treated as final values by elbehery95 · Pull Request #14745 · python/mypy

elbehery95 · 2023-02-20T19:03:56Z

Generated __native.c will not compile when mypyc is invoked against the following example.py:

from enum import IntEnum

class Color(IntEnum):
    GREEN = 1

assert Color.GREEN == 1

Error

...
build/__native.c: In function ‘CPyDef___top_level__’:
build/__native.c:358:17: error: ‘CPyStatic_Color___GREEN’ undeclared (first use in this function)
  358 |     cpy_r_r74 = CPyStatic_Color___GREEN;
...

Previously code only treated enum.Enum attributes as Final, I switched to using TypeInfo.is_enum instead of checking type full name against hard-coded string

Your feedback is appreciated

elbehery95 · 2023-02-20T20:15:20Z

~~Not sure why this is failing against 3.8 but passing against 3.7 and 3.9 !~~
probably related to the info in this commit python/cpython@8a4f085 and python/cpython#26654 (comment)

As per my understanding this seems to be an issue in cpython implementation that was fixed in later releases, but could not be backported to 3.8 as it was back then allowing security fixes only.

ichard26

Hey thanks for filing a PR. I'm a new reviewer so this in no way counts as a full review, but here's some initial feedback:

Typically we prefix commits and PRs focused on mypyc with [mypyc]. For example ...
- #14747
Could you test the function based syntax for IntEnum?
- Color = IntEnum("Color", ["GREEN"])
Add Fixes mypyc/mypyc#721 somewhere in your PR description so GitHub will automatically close the related bug report once this PR is merged.

IntEnum test is skipped on python3.8 due to python/cpython#26654

elbehery95 · 2023-02-21T20:20:46Z

Thanks so much @ichard26
I have updated the commit as well as the description.

Regarding the functional based syntax, I think it is not working correctly for both Enum and IntEnum for example the following code will not compile with mypy 1.0.1

import enum
FunctionalColor = enum.Enum("FunctionalColor", ["RED", "BLUE"])
assert FunctionalColor.BLUE == 2

Probably would require a separate patch

ichard26

Looks good except from the tests.

It's probably a good idea to also test Flag enums, although I'm not sure whether mypyc still supports Python 3.5 as a compilation target. You may have to version guard this one too.

StrEnum and IntFlag would be nice to test too, but those are new in Python 3.11 so I won't complain if you don't add them in.

add reference to bpo associated with skipping test on python 3.8

elbehery95 · 2023-02-24T20:01:54Z

Thanks so much @ichard26 for the feedback,

I have added fix for StrEnum build error.

And after trying to understand test structure more, I have added tests for StrEnum, Flag and IntFlag

Should title change to reflect `StrEnum fix as well?

~~I recall StrEnum was broken (even after applying this patch locally) so probably would require a separate fix.~~

E.g.

from enum import StrEnum
class ColorFlag(StrEnum):
    RED = "red"

Reports

build/__native.c: In function ‘CPyDef___top_level__’:
build/__native.c:524:18: error: ‘CPyModule_typing’ undeclared (first use in this function); did you mean ‘CPyModule_sys’?
  524 |     cpy_r_r107 = CPyModule_typing;
      |                  ^~~~~~~~~~~~~~~~
      |                  CPyModule_sys

~~Regarding Flag and IntFlag they seem to be working when using CLI, but for some reason tests report the following error~~

class ColorFlag(Flag):
    RED = auto()
    GREEN = auto()
    BLUE = auto()

purple = ColorFlag.RED | ColorFlag.BLUE
assert ColorFlag.RED in purple

Reports

E   mypy.errors.CompileError: native.py:42: error: Unsupported right operand type for in ("ColorFlag")  [operator]

~~I am not sure if tests invoke mypyc with special arguments that trigger this error or not~~

add tests for StrEnum, Flag and IntFlag

elbehery95 · 2023-02-25T12:02:00Z

@@ -467,12 +467,13 @@ def populate_non_ext_bases(builder: IRBuilder, cdef: ClassDef) -> Value:
    The tuple is passed to the metaclass constructor.


should docstring change in this case ?

elbehery95 · 2023-02-25T12:09:35Z

        if cls.fullname == "builtins.object":
            continue
-        if is_named_tuple and cls.fullname in (
+        if (is_named_tuple or is_enum) and cls.fullname in (


My thoughts about this change are as the following:

is_extension_class will return False for StrEnum as metaclass_type is set to enum.EnumMeta

mypy/mypyc/irbuild/util.py

Line 130 in 284142d

if cdef.info.metaclass_type and cdef.info.metaclass_type.type.fullname not in (

transform_class_def mentioned explicitly in comments that classes inheriting from enum will have non-extension class object created for them

mypy/mypyc/irbuild/classdef.py

Line 107 in 284142d

# decorated or inherit from Enum. Classes decorated with @trait do not

For some reason populate_non_ext_bases is explicitly checking for named tuple though any class inheriting say str or have a typing.Sequence in mro it will cause the error currently seen in building code with StrEnum

mypy/mypyc/irbuild/classdef.py

Line 475 in 284142d

if is_named_tuple and cls.fullname in (

Not really sure if checking against both Enums and Named tuples is generic enough or not.

ichard26 · 2023-03-08T17:43:15Z

Hey, I know that I've been away for too long, but you don't need to close your PR. Unfortunately, PR reviews are scarce around here. Things don't always move quickly or even at a decent pace. I'm sorry about all of this, your work is appreciated. You're welcome to reopen this, or if you'd prefer it, I'm happy to file a new PR with your commits (attributed to you only!) and try to get it landed eventually.

Dreamsorcerer · 2024-08-12T23:48:41Z

@ichard26 Did you end up reviving this PR?

ichard26 · 2024-08-13T02:29:18Z

Nope. I've also stepped back from contributing to mypyc as reviews were few and far between. Sorry.

ichard26 suggested changes Feb 21, 2023

View reviewed changes

[mypyc] fix IntEnum attributes not treated as final values

3779501

IntEnum test is skipped on python3.8 due to python/cpython#26654

elbehery95 force-pushed the fix_intEnum branch from 899c1d2 to 3779501 Compare February 21, 2023 20:13

elbehery95 changed the title ~~fix IntEnum attributes not treated as final values~~ [mypyc] fix IntEnum attributes not treated as final values Feb 21, 2023

ichard26 reviewed Feb 23, 2023

View reviewed changes

Comment thread mypyc/test-data/run-classes.test Outdated

Comment thread mypyc/test-data/run-classes.test Outdated

Comment thread mypyc/test-data/run-classes.test Outdated

refactor imports in enum test

24edc14

add reference to bpo associated with skipping test on python 3.8

fix StrEnum build error

8757343

add tests for StrEnum, Flag and IntFlag

elbehery95 commented Feb 25, 2023

View reviewed changes

elbehery95 requested a review from ichard26 March 5, 2023 14:23

elbehery95 closed this Mar 8, 2023

elbehery95 deleted the fix_intEnum branch March 8, 2023 08:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[mypyc] fix IntEnum attributes not treated as final values#14745

[mypyc] fix IntEnum attributes not treated as final values#14745
elbehery95 wants to merge 3 commits into
python:masterfrom
elbehery95:fix_intEnum

elbehery95 commented Feb 20, 2023 •

edited

Loading

Uh oh!

elbehery95 commented Feb 20, 2023 •

edited

Loading

Uh oh!

ichard26 left a comment

Uh oh!

elbehery95 commented Feb 21, 2023 •

edited

Loading

Uh oh!

ichard26 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elbehery95 commented Feb 24, 2023 •

edited

Loading

Uh oh!

elbehery95 Feb 25, 2023

Uh oh!

elbehery95 Feb 25, 2023

Uh oh!

ichard26 commented Mar 8, 2023

Uh oh!

Dreamsorcerer commented Aug 12, 2024

Uh oh!

ichard26 commented Aug 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -467,12 +467,13 @@ def populate_non_ext_bases(builder: IRBuilder, cdef: ClassDef) -> Value:
		The tuple is passed to the metaclass constructor.

Uh oh!

Uh oh!

Conversation

elbehery95 commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elbehery95 commented Feb 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ichard26 left a comment

Choose a reason for hiding this comment

Uh oh!

elbehery95 commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ichard26 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elbehery95 commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elbehery95 Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

elbehery95 Feb 25, 2023

Choose a reason for hiding this comment

Uh oh!

ichard26 commented Mar 8, 2023

Uh oh!

Dreamsorcerer commented Aug 12, 2024

Uh oh!

ichard26 commented Aug 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

elbehery95 commented Feb 20, 2023 •

edited

Loading

elbehery95 commented Feb 20, 2023 •

edited

Loading

elbehery95 commented Feb 21, 2023 •

edited

Loading

elbehery95 commented Feb 24, 2023 •

edited

Loading